Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[es-rollover] Use OTEL helpers for TLS config instead of tlscfg #6238

Merged
merged 9 commits into from
Nov 24, 2024
Merged

[es-rollover] Use OTEL helpers for TLS config instead of tlscfg #6238

merged 9 commits into from
Nov 24, 2024

Conversation

chahatsagarmain
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

How was this change tested?

Checklist

@chahatsagarmain
use otel config tls
4640d82 
Signed-off-by: chahatsagarmain <[email protected]>
@chahatsagarmain chahatsagarmain requested a review from a team as a code owner November 21, 2024 22:48
@chahatsagarmain chahatsagarmain marked this pull request as draft November 21, 2024 22:48
@chahatsagarmain
Copy link
Contributor Author

chahatsagarmain commented Nov 21, 2024 •
edited
Loading

@yurishkuro i have made some changes here by replacing tlscfg with configtls , if its in the right direction i can move on for further changes .

@codecov Codecov
Copy link

codecov bot commented Nov 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.44%. Comparing base (f6899b0) to head (f8e6fc2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/es-rollover/app/flags.go 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6238      +/-   ##
==========================================
- Coverage   96.46%   96.44%   -0.02%     
==========================================
  Files         355      355              
  Lines       20149    20150       +1     
==========================================
- Hits        19436    19433       -3     
- Misses        526      529       +3     
- Partials      187      188       +1
Flag Coverage Δ
badger_v1 8.31% <ø> (ø)
badger_v2 1.67% <ø> (ø)
cassandra-4.x-v1 14.39% <ø> (ø)
cassandra-4.x-v2 1.61% <ø> (ø)
cassandra-5.x-v1 14.39% <ø> (ø)
cassandra-5.x-v2 1.61% <ø> (ø)
elasticsearch-6.x-v1 18.60% <ø> (ø)
elasticsearch-7.x-v1 18.68% <ø> (+<0.01%) ⬆️
elasticsearch-8.x-v1 18.84% <ø> (-0.01%) ⬇️
elasticsearch-8.x-v2 1.66% <ø> (ø)
grpc_v1 9.44% <ø> (ø)
grpc_v2 6.98% <ø> (ø)
kafka-v1 8.88% <ø> (ø)
kafka-v2 1.67% <ø> (ø)
memory_v2 1.67% <ø> (ø)
opensearch-1.x-v1 18.73% <ø> (ø)
opensearch-2.x-v1 18.72% <ø> (-0.01%) ⬇️
opensearch-2.x-v2 1.66% <ø> (ø)
tailsampling-processor 0.46% <ø> (ø)
unittests 95.35% <77.77%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@chahatsagarmain
removed redudant code
b52c388 
Signed-off-by: chahatsagarmain <[email protected]>
@chahatsagarmain
fix
830f11c 
Signed-off-by: chahatsagarmain <[email protected]>
@chahatsagarmain chahatsagarmain marked this pull request as ready for review November 22, 2024 19:45
@dosubot dosubot bot added the area/otel label Nov 22, 2024
yurishkuro
yurishkuro reviewed Nov 22, 2024
View reviewed changes
cmd/es-rollover/app/actions.go Outdated Show resolved Hide resolved
cmd/es-rollover/app/actions.go Outdated Show resolved Hide resolved
cmd/es-rollover/app/actions.go Outdated Show resolved Hide resolved
cmd/es-rollover/app/flags.go Outdated Show resolved Hide resolved
cmd/es-rollover/main.go Outdated Show resolved Hide resolved
cmd/es-rollover/main.go Outdated Show resolved Hide resolved
cmd/es-rollover/main.go Outdated Show resolved Hide resolved
cmd/es-rollover/main.go Outdated Show resolved Hide resolved
cmd/es-rollover/main.go Outdated Show resolved Hide resolved
cmd/es-rollover/app/flags.go Outdated Show resolved Hide resolved
@chahatsagarmain
removed unused struct
70b1890 
Signed-off-by: chahatsagarmain <[email protected]>
@chahatsagarmain
Copy link
Contributor Author

@yurishkuro I have removed the unnecessary client config struct and added ClientConfig to Config but it seems actions_test unit is failing maybe because i am not properly handling flags in flags.go

@chahatsagarmain
fix
64b42cc 
Signed-off-by: chahatsagarmain <[email protected]>
@chahatsagarmain
panic in initviper
4b42e6d 
Signed-off-by: chahatsagarmain <[email protected]>
chahatsagarmain and others added 2 commits November 25, 2024 01:36
@chahatsagarmain
resolved comments
578a34e 
Signed-off-by: chahatsagarmain <[email protected]>
@yurishkuro
use viperize
d2394b4 
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro yurishkuro changed the title Use OTEL helpers for TLS config instead of tlscfg in es-rollover [es-rollover] Use OTEL helpers for TLS config instead of tlscfg Nov 24, 2024
@yurishkuro yurishkuro enabled auto-merge (squash) November 24, 2024 20:16
@yurishkuro yurishkuro disabled auto-merge November 24, 2024 20:32
@yurishkuro yurishkuro merged commit 4d1c7d8 into jaegertracing:main Nov 24, 2024
51 of 52 checks passed
@chahatsagarmain chahatsagarmain deleted the es-rollover-tlscfg branch November 24, 2024 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

@jkowall jkowall Awaiting requested review from jkowall jkowall is a code owner automatically assigned from jaegertracing/jaeger-maintainers

@mahadzaryab1 mahadzaryab1 Awaiting requested review from mahadzaryab1

Assignees
No one assigned
Labels
area/otel changelog:bugfix-or-minor-feature storage/elasticsearch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chahatsagarmain @yurishkuro @mahadzaryab1